Update NUnit to v4 by Trenly · Pull Request #6287 · microsoft/winget-cli

Trenly · 2026-06-16T02:41:50Z

📖 Description

Upgrade the AppInstallerCLIE2ETests suite to NUnit 4 and modernize the supporting test code to the NUnit 4 constraint model. This lands on the latest available NUnit 4.x release in NuGet (4.6.1)

The update converts the E2E suite off NUnit 3 classic assertions, fixes the delegate-based assertion overloads required by NUnit 4, and carries the package version change through the affected test projects that use centralized package management.

I opted for full migration instead of the simpler Directory.Build.props change as I expect this will be a one-time migration and I generally believe that using the newer format will be more performant in the future (plus I had the extra tokens to burn, so why not)

🔗 References

Split from Update NuGet Packages #6258
NUnit 4 migration guide: https://docs.nunit.org/articles/nunit/release-notes/Nunit4.0-MigrationGuide.html

🔍 Validation

Search-based verification confirmed the old NUnit 3 classic assertions were removed from src/AppInstallerCLIE2ETests.

✅ Checklist

Signed the Contributor License Agreement
Linked to an issue
Updated Release Notes (if applicable)
Updated documentation (if applicable)
Updated Copilot instructions (if build, architecture, or conventions changed)

📋 Issue Type

Bug fix
Feature
Task

Microsoft Reviewers: Open in CodeFlow

florelis

I think it would be good to use some of the native constraints in more places. Like
Assert.That(x, Is.EqualTo(0)) -> Assert.That(x, Is.Zero)
Assert.That(s.Contains("substr"), Is.True) -> Assert.That(s, Contains.Substring("substr"))
Assert.That(File.Exists(f), Is.True) -> Assert.That(f, Does.Exist)

florelis · 2026-07-14T00:54:41Z

-            Assert.IsFalse(output.Settings.ContainsKey(NotAnAdminSettingName));
+            Assert.That(output, Is.Not.Null);
+            Assert.That(output.Settings, Is.Not.Null);
+            Assert.That(output.Settings.ContainsKey(LocalManifestFiles), Is.True);


Depending on the type of Settings, maybe this could be

Assert.That(output.Settings, Contains.Key(LocalManifestFiles));

It could be, but I wanted to keep all the tests consistent with how they handle JSON. I'm happy to update across all the files if that is preferred, but don't see a lot of benefit in that. Of course, it may also be easier to take that as a follow on to minimize the amount of time needed for a re-review on this PR

florelis · 2026-07-14T18:52:56Z

/azp run

azure-pipelines · 2026-07-14T18:53:06Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

Upgrades the AppInstallerCLIE2ETests suite to NUnit 4 and migrates assertions throughout the E2E test codebase to the NUnit constraint model, while also updating centrally-managed NuGet package versions.

Changes:

Updated central package versions for nunit (and also NUnit3TestAdapter / Microsoft.ApplicationInsights) in src/Directory.Packages.props.
Converted E2E tests from classic assertions (Assert.True/False/AreEqual/Throws*) to Assert.That(..., constraint) patterns compatible with NUnit 4.
Minor test cleanups (e.g., removing unused using directives, resolving type name ambiguity via alias).

Reviewed changes

Copilot reviewed 56 out of 56 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/WinGetUtilInterop.UnitTests/WinGetUtilInterop.UnitTests.csproj	Adds a package reference (currently appears unnecessary).
src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationProcessorTelemetryTests.cs	Adds a `TelemetryEvent` using-alias to disambiguate types.
src/Directory.Packages.props	Updates centrally-managed package versions (includes NUnit and other packages).
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilSQLiteIndex.cs	Migrates WinGetUtil SQLite index tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs	Migrates WinGetUtil manifest tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilLog.cs	Migrates logging test assertions to constraint-based style.
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilInstallerMetadataCollection.cs	Migrates installer metadata tests to constraint-based assertions (incl. exception assertions).
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilDownload.cs	Removes unused `System.Linq` and migrates assertions to constraints.
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilCompareVersions.cs	Migrates version compare test assertion to constraints.
src/AppInstallerCLIE2ETests/ValidateCommand.cs	Migrates validate command tests to constraint-based assertions (incl. non-ASCII manifest filename).
src/AppInstallerCLIE2ETests/UpgradeCommand.cs	Migrates upgrade command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/UninstallCommand.cs	Migrates uninstall command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ShowCommand.cs	Migrates show command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/SetUpFixture.cs	Migrates setup fixture assertions to constraint-based style.
src/AppInstallerCLIE2ETests/SearchCommand.cs	Migrates search command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ResumeCommand.cs	Migrates resume tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/RepairCommand.cs	Migrates repair command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Pinning.cs	Migrates pinning behavior tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ListCommand.cs	Migrates list command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/UpgradeInterop.cs	Migrates interop upgrade tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/UninstallInterop.cs	Migrates interop uninstall tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/Shutdown.cs	Migrates shutdown interop tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/RepairInterop.cs	Migrates repair interop tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs	Migrates package catalog interop tests (incl. async exception assertions) to constraint style.
src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs	Migrates group policy interop tests; updates exception assertion patterns.
src/AppInstallerCLIE2ETests/Interop/FindPackagesInterop.cs	Migrates find-packages interop tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/DownloadInterop.cs	Migrates download interop tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/ConnectionValidationInterop.cs	Migrates connection validation tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/BaseInterop.cs	Migrates base interop assertion(s); file header encoding changed.
src/AppInstallerCLIE2ETests/InprocTestbedTests.cs	Migrates in-proc testbed exit-code assertion to constraints.
src/AppInstallerCLIE2ETests/ImportCommand.cs	Migrates import command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs	Migrates shared E2E helper assertions to constraint-based style.
src/AppInstallerCLIE2ETests/HashCommand.cs	Migrates hash command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/GroupPolicy.cs	Migrates CLI group policy tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/FontCommand.cs	Migrates font command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/FeaturesCommand.cs	Migrates features command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ErrorCommand.cs	Migrates error command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs	Migrates DSCv3 user settings resource tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/DSCv3SourceResourceCommand.cs	Migrates DSCv3 source resource tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/DSCv3ResourceTestBase.cs	Migrates DSCv3 shared test utilities to constraint-based assertions.
src/AppInstallerCLIE2ETests/DSCv3PackageResourceCommand.cs	Migrates DSCv3 package resource tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/DSCv3AdminSettingsResourceCommand.cs	Migrates DSCv3 admin settings resource tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/DownloadCommand.cs	Migrates download command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ConfigureTestCommand.cs	Migrates configure test command validations to constraint-based assertions.
src/AppInstallerCLIE2ETests/ConfigureShowCommand.cs	Migrates configure show command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ConfigureProcessorPathCommand.cs	Migrates configure processor-path tests and regex assertion usage to constraints.
src/AppInstallerCLIE2ETests/ConfigureListCommand.cs	Migrates configure list command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs	Migrates configure export command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/AppShutdownTests.cs	Migrates app shutdown tests to constraint-based assertions.
doc/ReleaseNotes.md	Updates release notes with NUnit v4 migration entry and bumps “New in” header.

    <PackageReference Include="Moq" />
+    <PackageReference Include="System.Text.Json" />
    <PackageReference Include="xunit" />


florelis · 2026-07-14T19:02:45Z

I don't think any of the Copilot comments need to be resolved. The BOM issue I would want to have fixed, but I think that would be a repo-level thing, like the normalization of line endings we did.

Trenly · 2026-07-14T19:04:55Z

I don't think any of the Copilot comments need to be resolved. The BOM issue I would want to have fixed, but I think that would be a repo-level thing, like the normalization of line endings we did.

Reading through them, I agree. If the pipelines fail, I'll address the two that I've left open. I've replied to the others with my justification for resolving them

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

florelis · 2026-07-14T19:58:17Z

/azp run

azure-pipelines · 2026-07-14T19:58:28Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Trenly · 2026-07-14T21:38:32Z

This looks fun . . .

##[error]TearDown failed for test fixture AppInstallerCLIE2ETests.SetUpFixture
##[error]One or more child tests were ignored
##[error]TearDown : System.IO.IOException : The file 'C:\Users\VssAdministrator\AppData\Local\Temp\E2ETestLogs\Unpackaged\WinGetCOM-2026-07-14-21-20-31.670.log' already exists.
##[error]StackTrace: --TearDown
##[error]   at System.IO.FileSystem.CopyFile(String sourceFullPath, String destFullPath, Boolean overwrite)
##[error]   at AppInstallerCLIE2ETests.Helpers.TestCommon.CopyDirectory(String sourceDirName, String destDirName) in D:\a\1\s\src\AppInstallerCLIE2ETests\Helpers\TestCommon.cs:line 1070
##[error]   at AppInstallerCLIE2ETests.Helpers.TestCommon.PublishE2ETestLogs() in D:\a\1\s\src\AppInstallerCLIE2ETests\Helpers\TestCommon.cs:line 545
##[error]   at AppInstallerCLIE2ETests.SetUpFixture.TearDown() in D:\a\1\s\src\AppInstallerCLIE2ETests\SetUpFixture.cs:line 84
##[error]   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
##[error]   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Will trace down the root cause when I get home. However . . . looks like the tests themselves are passing, so that's promising at least

Total tests: 100
     Passed: 94
    Skipped: 6

Trenly · 2026-07-15T14:03:26Z

@florelis - This is an already existing defect which happens in Azure because the log files currently have the overwrite flag set to false. When a PR is run multiple times in Azure the files already exist so this copy fails. You can see this bug in the logs from #6365.

It appears that this has always been a bug in the test adapter and it was fixed on the newer version of the NuGet package, which is why it is causing a fault now.

I would like some advice on how to approach the fix. It feels like this may be a case where just setting overwrite to true is acceptable, but didn't want to do that without confirming first

florelis · 2026-07-15T17:51:16Z

I don't think it has to do with the PR running multiple times. I believe it happens because we copy the winget log folder to the output directory at multiple times within the same run, so we end up trying to copy the same file again. And I'd guess it started happening because the previous adapter would ignore errors during teardown.

Overwriting should be fine since it should be the same file, even the timestamp has to match. We could add the parameter to CopyDirectory() so we can limit it to only the places where we are copying the logs. Worst case, it can't be worse than what we have today - we would still be missing one of the logs files, it's just the older one that is missing now.

A better solution could be to prevent the double copy from happening to begin with. I believe this could be prevented if we cleared the winget logs during the OneTimeSetUp. So, each time we call the E2E tests with different options, we would only need to copy what that test run generated.

…UnitUpdate

Trenly · 2026-07-15T18:32:32Z

I don't think it has to do with the PR running multiple times. I believe it happens because we copy the winget log folder to the output directory at multiple times within the same run, so we end up trying to copy the same file again. And I'd guess it started happening because the previous adapter would ignore errors during teardown.

I see . . . .That makes sense

Overwriting should be fine since it should be the same file, even the timestamp has to match. We could add the parameter to CopyDirectory() so we can limit it to only the places where we are copying the logs. Worst case, it can't be worse than what we have today - we would still be missing one of the logs files, it's just the older one that is missing now.

100% agree. I'll take this path for simplicity

A better solution could be to prevent the double copy from happening to begin with. I believe this could be prevented if we cleared the winget logs during the OneTimeSetUp. So, each time we call the E2E tests with different options, we would only need to copy what that test run generated.

Sounds like this will be another great follow on issue which I can file and look at

florelis · 2026-07-15T18:48:12Z

/azp run

azure-pipelines · 2026-07-15T18:48:28Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Trenly · 2026-07-15T21:10:15Z

Ah, finally, a green checkmark

Trenly added 3 commits June 15, 2026 21:36

Update NUnit to v4

5db9b06

Bump version of ApplicationInsights as required by NUnit

86a3049

Release Notes

d31b42f

Trenly marked this pull request as ready for review June 16, 2026 03:03

Trenly requested a review from a team as a code owner June 16, 2026 03:03

Trenly added 2 commits June 23, 2026 11:05

Merge branch 'master' into NUnitUpdate

792ee6e

Merge remote-tracking branch 'upstream/master' into HardlinkAliases

a6efb73

florelis reviewed Jul 14, 2026

View reviewed changes

Trenly added 5 commits July 13, 2026 20:37

Merge remote-tracking branch 'upstream/master' into NUnitUpdate

019aeb3

Use builtins

1d95fa0

Use collection native

907b828

Address addtional comments

97b0050

Use older style for group policy hresult

6e91fb2

florelis previously approved these changes Jul 14, 2026

View reviewed changes

florelis requested a review from Copilot July 14, 2026 18:52

Copilot started reviewing on behalf of florelis July 14, 2026 18:53 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Fix BOM Mark which seems to be causing build issue

3aae433

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Trenly dismissed florelis’s stale review via 3aae433 July 14, 2026 19:56

Trenly added 2 commits July 15, 2026 13:30

Allow overwrite

36d4acf

Merge remote-tracking branch 'refs/remotes/origin/NUnitUpdate' into N…

93d4b34

…UnitUpdate

Trenly requested a review from florelis July 15, 2026 18:32

florelis approved these changes Jul 15, 2026

View reviewed changes

florelis merged commit e2f795f into microsoft:master Jul 15, 2026
9 checks passed

Trenly deleted the NUnitUpdate branch July 15, 2026 21:36

This was referenced Jul 15, 2026

Improve Pipeline Handling of Copying Test Results #6368

Open

Use NUnit built-ins for Settings Tests #6369

Closed

Add contstraints to simplify E2E tests #6370

Open

Uh oh!

Conversation

Trenly commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📖 Description

🔗 References

🔍 Validation

✅ Checklist

📋 Issue Type

Microsoft Reviewers: Open in CodeFlow

Uh oh!

florelis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

florelis Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Trenly Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

florelis commented Jul 14, 2026

Uh oh!

azure-pipelines Bot commented Jul 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

florelis commented Jul 14, 2026

Uh oh!

Trenly commented Jul 14, 2026

Uh oh!

florelis commented Jul 14, 2026

Uh oh!

azure-pipelines Bot commented Jul 14, 2026

Uh oh!

Trenly commented Jul 14, 2026

Uh oh!

Trenly commented Jul 15, 2026

Uh oh!

florelis commented Jul 15, 2026

Uh oh!

Trenly commented Jul 15, 2026

Uh oh!

florelis commented Jul 15, 2026

Uh oh!

azure-pipelines Bot commented Jul 15, 2026

Uh oh!

Trenly commented Jul 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Trenly commented Jun 16, 2026 •

edited

Loading